[Cases] Use the new internal users API in the UI #150432

cnasikas · 2023-02-07T13:45:03Z

Summary

This PR updates the UI to use the new users API introduced in #149663. I changed the backend response to accommodate the needs of the UI.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2023-02-08T16:07:04Z

Pinging @elastic/response-ops (Team:ResponseOps)

elasticmachine · 2023-02-08T16:07:06Z

Pinging @elastic/response-ops-cases (Feature:Cases)

jonathan-buttner

Code changes look good, it looks like the UI is only showing the emails now though

Question: Does this PR switch the participants to include the currently assigned users? In the currently assigned users aren't show in the participants list.

jonathan-buttner · 2023-02-08T21:01:08Z

x-pack/plugins/cases/public/components/case_view/components/case_view_activity.tsx

-            userProfiles={userProfiles}
-          />
-          {userActionsData?.participants ? (
+          {caseUsers?.reporter ? (


Should we show the Unknown user if we don't have the reporter instead of hiding the list?

I suppose another option would be to fallback to the case data right?

Thanks! I will fallback to the case data.

jonathan-buttner · 2023-02-08T21:06:42Z

x-pack/plugins/cases/public/containers/mock.ts

@@ -957,3 +958,85 @@ export const getPersistableStateAttachment = (
    ...viewObject,
  }),
 });
+
+export const getCaseUsersMockResponse = (): CaseUsers => {
+  return {


Should we have a mock response that only has the profileUid?

js-jankisalvi · 2023-02-09T10:28:22Z

x-pack/plugins/cases/public/components/case_view/components/case_view_activity.tsx

+    ...users.assignees,
+    ...users.participants,
+    users.reporter,
+    ...users.unassignedUsers,


Do we need to have uniq users array like it was before in
const uidsToRetrieve = uniq([...userActionProfileUids, ...assignees]);?

I think the Map below guarantees that only unique profiles will be added. The map will contain all valid user profiles.

js-jankisalvi · 2023-02-09T10:40:12Z

x-pack/plugins/cases/public/containers/mock.ts

+        },
+      },
+    ],
+    unassignedUsers: [


This might be a stupid question 😄
What is the difference between unassignedUsers and Participants?

There are no stupid questions 🙂! Participants are all users that did an action in the case. For example, edit a description, a comment, etc. Now, if you assign a user to a case a user action will be created containing the uid of the user. Same if you remove an assignee (unassigned user). We need the profile information for that users.

ahh, got it. Thanks 🙂

js-jankisalvi · 2023-02-09T10:41:09Z

x-pack/plugins/cases/public/containers/use_find_case_user_actions.test.tsx

@@ -80,137 +74,4 @@ describe('UseFindCaseUserActions', () => {
    expect(spy).toHaveBeenCalledWith(basicCase.id, expect.any(AbortSignal));
    expect(addError).toHaveBeenCalled();
  });
-
-  describe('getProfileUids', () => {


Yay!! 🎉 🙌

js-jankisalvi · 2023-02-09T11:30:14Z

Code changes look good, it looks like the UI is only showing the emails now though

I also see the same behaviour. Showing username instead of email will look cleaner

cnasikas · 2023-02-09T12:19:27Z

Code changes look good, it looks like the UI is only showing the emails now though

I also see the same behaviour. Showing username instead of email will look cleaner

Damn, Rigid Rodent did it again. Thanks, both of you for finding the bug. I should be more careful. I pushed the fix and added many unit tests to check user profile variations. The bug occurred because I converted the response to camel case when the UI expected it to be in snake case. That was the format returned by the bulkGet of the security plugin. I slightly modified the backend response to be more inline with the response from the security plugin to avoid in the future these kind of bugs. Could you please also test that the avatar is displayed correctly if you change it to be an image or a different color? You can do that by going to:

Question: Does this PR switch the participants to include the currently assigned users? In the currently assigned users aren't show in the participants list.

No, but it should. I fixed it in this PR.

js-jankisalvi · 2023-02-09T13:34:21Z

Damn, Rigid Rodent did it again. Thanks, both of you for finding the bug. I should be more careful. I pushed the fix and added many unit tests to check user profile variations. The bug occurred because I converted the response to camel case when the UI expected it to be in snake case. That was the format returned by the bulkGet of the security plugin. I slightly modified the backend response to be more inline with the response from the security plugin to avoid in the future these kind of bugs. Could you please also test that the avatar is displayed correctly if you change it to be an image or a different color? You can do that by going to:

Updated user profile with image, Avatar icon is displayed:

Tested again. Looks good now. (Note: Reporter shows old profile image of elastic user)

js-jankisalvi

Verified locally, looks great 👍 🎉

jonathan-buttner · 2023-02-09T14:19:22Z

x-pack/plugins/cases/common/api/user.ts

  }),
+  rt.partial({ uid: rt.string }),


I wonder if there's a way we can tie this implementation with the security plugin 🤔 My concern is that if they make a breaking change to the response structure and make the changes throughout the code base, this won't show up as needing to be changed and it will break the UI.

Here's an idea, how about we add an integration test that does a bulk get using the security API and then does a decode using this schema? Or something like that, that way if there's a breaking change at least we'll get a failing test.

Amazing idea! I was thinking about the same but I could not find a good way to do it.

jonathan-buttner · 2023-02-09T14:27:35Z

x-pack/plugins/cases/public/components/case_view/components/case_view_activity.tsx

          {caseUsers?.participants ? (
            <UserList
              dataTestSubj="case-view-user-list-participants"
              theCase={caseData}
              headline={i18n.PARTICIPANTS}
              loading={isLoadingUserActions}
-              users={caseUsers.participants}
+              users={[...caseUsers.participants, ...caseUsers.assignees]}


Does the ternary operator on line 245 need to be updated to reflect the assignees too?

How about we combine the array above this component and just check to see if it's not empty? Although is it even possible to have assignees but not a participant 🤔 that'd mean we don't have the reporter's information I suppose. Either probably best to check just in case the backend has a bug.

jonathan-buttner

Great work!

...st/cases_api_integration/security_and_spaces/tests/common/internal/user_actions_get_users.ts

kibana-ci · 2023-02-11T10:10:30Z

💚 Build Succeeded

Buildkite Build
Commit: 3475370

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cases`	595	596	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`cases`	394.1KB	394.2KB	+82.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`cases`	130.0KB	130.1KB	+79.0B

History

💔 Build #107346 failed b7d8711
💚 Build #106841 succeeded c9be240
💔 Build #106830 failed a2ca4c6
💔 Build #106487 failed d73d7fe
💚 Build #106064 succeeded d943837

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @cnasikas

cnasikas added 24 commits January 27, 2023 10:22

Create endpoint

0052783

Add integration tests

dc76d81

Fix types

da0b433

Merge branch 'main' into user_actions_users_api

9e32bbf

Fix SO test

c80ca5f

Change response structure

3cedccf

Merge branch 'main' into user_actions_users_api

1327690

Fix imports

1479e88

Improvements

1b2804e

Tests for basic license

74a1b5b

Merge branch 'main' into user_actions_users_api

5765a73

Merge branch 'main' into user_actions_users_api

8d042ac

PR feedback

b69eb79

Better structure

68802de

PR feedback

8cf7876

Fix tests

c02905d

Merge branch 'main' into user_actions_users_api

2163590

Better naming

36d5548

Create useGetCaseUsers hook

39217c2

Rename ElasticUser to CaseUser

4f1ffbb

Merge branch 'main' into use_users_api_ui

f1f1f1c

Merge branch 'main' into use_users_api_ui

b3a9716

Change to the new API

ca455b6

Fix tests & types

d943837

cnasikas added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.8.0 labels Feb 7, 2023

cnasikas self-assigned this Feb 7, 2023

cnasikas mentioned this pull request Feb 7, 2023

[Cases] Filter & paginate cases activity #140096

Closed

9 tasks

cnasikas requested a review from a team as a code owner February 8, 2023 16:06

jonathan-buttner reviewed Feb 8, 2023

View reviewed changes

cnasikas added 4 commits February 9, 2023 10:54

Merge branch 'main' into use_users_api_ui

f8e07f9

Fix display name when rendering users

e8193ff

Add assignees as participants

377cd88

Fallback reporter

83331e4

js-jankisalvi reviewed Feb 9, 2023

View reviewed changes

Add unit tests

d8e94d8

cnasikas force-pushed the use_users_api_ui branch from f15e24e to d8e94d8 Compare February 9, 2023 12:16

cnasikas requested review from jonathan-buttner and js-jankisalvi February 9, 2023 12:20

cnasikas added 2 commits February 9, 2023 14:27

Fix tests

a2ca4c6

Fix types

c9be240

js-jankisalvi approved these changes Feb 9, 2023

View reviewed changes

jonathan-buttner reviewed Feb 9, 2023

View reviewed changes

cnasikas added 2 commits February 10, 2023 17:43

PR feedback

1854fd8

Sort user list

b7d8711

jonathan-buttner approved these changes Feb 10, 2023

View reviewed changes

...st/cases_api_integration/security_and_spaces/tests/common/internal/user_actions_get_users.ts Outdated Show resolved Hide resolved

cnasikas added 2 commits February 11, 2023 10:33

Merge branch 'main' into use_users_api_ui

e439622

Fix tests

3475370

cnasikas enabled auto-merge (squash) February 11, 2023 09:00

cnasikas merged commit 4f95b7c into elastic:main Feb 11, 2023

cnasikas deleted the use_users_api_ui branch February 11, 2023 10:10

kibanamachine added the backport:skip This commit does not require backporting label Feb 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cases] Use the new internal users API in the UI #150432

[Cases] Use the new internal users API in the UI #150432

cnasikas commented Feb 7, 2023 •

edited

Loading

elasticmachine commented Feb 8, 2023

elasticmachine commented Feb 8, 2023

jonathan-buttner left a comment

jonathan-buttner Feb 8, 2023

cnasikas Feb 9, 2023

jonathan-buttner Feb 8, 2023

js-jankisalvi Feb 9, 2023

cnasikas Feb 9, 2023

js-jankisalvi Feb 9, 2023

cnasikas Feb 9, 2023

js-jankisalvi Feb 9, 2023

js-jankisalvi Feb 9, 2023

js-jankisalvi commented Feb 9, 2023

cnasikas commented Feb 9, 2023 •

edited

Loading

js-jankisalvi commented Feb 9, 2023

js-jankisalvi left a comment

jonathan-buttner Feb 9, 2023

cnasikas Feb 9, 2023

jonathan-buttner Feb 9, 2023

jonathan-buttner left a comment

kibana-ci commented Feb 11, 2023

[Cases] Use the new internal users API in the UI #150432

[Cases] Use the new internal users API in the UI #150432

Conversation

cnasikas commented Feb 7, 2023 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Feb 8, 2023

elasticmachine commented Feb 8, 2023

jonathan-buttner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

js-jankisalvi commented Feb 9, 2023

cnasikas commented Feb 9, 2023 • edited Loading

js-jankisalvi commented Feb 9, 2023

js-jankisalvi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathan-buttner left a comment

Choose a reason for hiding this comment

kibana-ci commented Feb 11, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

cnasikas commented Feb 7, 2023 •

edited

Loading

cnasikas commented Feb 9, 2023 •

edited

Loading